feat: wiggle and silhouette stacks #751

markov00 · 2020-07-15T13:08:09Z

Summary

This PR adds the wiggle and silhouette stacks mode as available in vislib.
t is also to fix fit functions for stacked charts #450

Configuring a stackMode in a stacked area or bar chart will change the baseline offset used to render each group of stacks. StackMode are:

export const StackMode = Object.freeze({
  /** Applies a zero baseline and normalizes the values for each point such that the topline is always one. */
  Percentage: 'percentage' as const,
  /** Shifts the baseline so as to minimize the weighted wiggle of layers. */
  Wiggle: 'wiggle' as const,
  /** Shifts the baseline down such that the center of the streamgraph is always at zero. */
  Silhouette: 'silhouette' as const,
});

Applied to a stacked area chart it can be used to stream graphs

close #715

Additional changes

Fit functions are now working also on stacked charts
close Fit functions for stacked line and area charts #450
The legend extra value is now displayed, on-demand, only on charts with a continuous x-axis
fix Don't display legend extra values when the x scale is ordinal #766
The PR adjust slightly the way fitted data is represented in the case of null values and no fit option specified. It renders an empty area segment as in the following screenshot

The PR also adjust how we fit the chart to the data domain: for zero-based charts (bar and area charts) the fit option doesn't have effects, thus the changes on the screenshots.
The PR finally adjust the clipped ranges for ordinal charts, where the lines where wrongly clipped due to a missing translate call when rendering.
Some tests are converted to functional tests, instead of unit tests.

BREAKING CHANGE:

PointStyleAccessor and BarStyleAccessor now calls the method with a DataSeriesDatum as first argument instead of a RawDataSeriesDatum
stackAsPercentage prop on BarsSpec and AreaSpec is replaced by stackMode that accept one StackMode.

Checklist

Any consumer-facing exports were added to src/index.ts (and stories only import from ../src except for test data & storybook)
Proper documentation or storybook story was added for features that require explanation or tutorials
Unit tests were updated or added to match the most common scenarios

formatNonStackedDataSeriesValues and formatStackedDataSeriesValues are always called with a false value of 'scaleToExtent'. Because of this, this commit removes that parameter from the call. It also clean up and refactor some test into integration test

BREAKING CHANGE: PointStyleAccessor and BarStyleAccessor now calls the method with a DataSeriesDatum not with a RawDataSeriesDatum

codecov-commenter · 2020-07-22T15:56:38Z

Codecov Report

Merging #751 into master will increase coverage by 0.21%.
The diff coverage is 85.36%.

@@            Coverage Diff             @@
##           master     #751      +/-   ##
==========================================
+ Coverage   74.33%   74.54%   +0.21%     
==========================================
  Files         269      285      +16     
  Lines        9303     9578     +275     
  Branches     1936     1945       +9     
==========================================
+ Hits         6915     7140     +225     
- Misses       2383     2427      +44     
- Partials        5       11       +6

Flag	Coverage Δ
#unittests	`74.01% <84.71%> (-0.32%)`	⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files	Coverage Δ
src/chart_types/xy_chart/renderer/canvas/areas.ts	`40.00% <0.00%> (ø)`
..._types/xy_chart/renderer/canvas/primitives/path.ts	`9.43% <0.00%> (-1.21%)`	⬇️
src/chart_types/xy_chart/specs/bar_series.tsx	`100.00% <ø> (ø)`
src/components/tooltip/tooltip.tsx	`70.00% <ø> (-0.33%)`	⬇️
src/mocks/series/data.ts	`100.00% <ø> (ø)`
src/mocks/specs/specs.ts	`77.01% <ø> (ø)`
src/renderers/canvas/index.ts	`44.00% <0.00%> (ø)`
src/utils/geometry.ts	`100.00% <ø> (ø)`
...chart_types/xy_chart/utils/stacked_series_utils.ts	`86.11% <85.29%> (-11.87%)`	⬇️
src/chart_types/xy_chart/state/utils/utils.ts	`92.57% <90.69%> (-1.20%)`	⬇️
... and 30 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 4455281...63b29a6. Read the comment docs.

the `stackMode` was introduced to apply percentage, wiggle and silhouette offset for stacking charts. BREAKING CHANGE: `stackAsPercentage` prop was replaced by `stackMode` that accept one `StackModes`. fix elastic#715

nickofthyme

Code changes look good to me, still reviewing behavior and visual screenshot changes. This was lot fewer actual src code changes than I was expecting seeing the +6,975 −3,995.

After looking at the streamgraph example. I think we should change the tooltip highlighting behavior in a future PR. Maybe this is normal but to me having the geometry point at the top as the area is strange. I feel these chart types are more about the difference in relative height to where the top points are irrelevant. Maybe we just hide points altogether for streamgraphs charts and never highlight the tooltip values.

src/chart_types/xy_chart/renderer/canvas/primitives/path.ts

src/chart_types/xy_chart/rendering/rendering.ts

src/utils/data/babynames.ts

src/utils/domain.ts

stories/area/14_stacked_band.tsx

stories/area/16_test_stacked_with_missing.tsx

stories/streamgraph/1_basic.tsx

src/chart_types/xy_chart/utils/specs.ts

src/chart_types/xy_chart/rendering/rendering.test.ts

nickofthyme

I notice you changed the extra value appearance on many screenshots. I think we should try to keep sweeping visual changes like this to small PRs. That said, I verified all of the screenshots look ok with the changes, so I'm fine with those changes for this PR.

Some screenshots I couldn't see the difference. For instance the none type fitting function screenshots like integration/tests/__image_snapshots__/mixed-stories-test-ts-mixed-series-stories-fitting-functions-area-charts-end-value-set-to-2-should-display-correct-fit-for-type-none-1-snap.png as well as integration/tests/__image_snapshots__/all-test-ts-baseline-visual-tests-for-all-stories-legend-display-values-in-legend-elements-visually-looks-correct-1-snap.png. Are these just minor canvas rendering changes?

src/chart_types/xy_chart/state/utils/types.ts

src/mocks/series/series.ts

src/mocks/series/utils.ts

src/chart_types/xy_chart/utils/fit_function.ts

markov00 · 2020-07-29T10:42:45Z

@nickofthyme

Some screenshots I couldn't see the difference. For instance the none type fitting function screenshots like integration/tests/image_snapshots/mixed-stories-test-ts-mixed-series-stories-fitting-functions-area-charts-end-value-set-to-2-should-display-correct-fit-for-type-none-1-snap.png as well as integration/tests/image_snapshots/all-test-ts-baseline-visual-tests-for-all-stories-legend-display-values-in-legend-elements-visually-looks-correct-1-snap.png. Are these just minor canvas rendering changes?

Exactly, this probably is caused or by the changes to fix the dashed line misaligned, or due to smaller changes on the clipping ranges

I notice you changed the extra value appearance on many screenshots. I think we should try to keep sweeping visual changes like this to small PRs. That said, I verified all of the screenshots look ok with the changes, so I'm fine with those changes for this PR.

My bad I'm sorry for that, whenever something wrong caches my eyes I preferred to fix that as soon as possible to avoid forget it later... I will do better on my next PR

nickofthyme · 2020-07-29T13:34:33Z

My bad I'm sorry for that, whenever something wrong caches my eyes I preferred to fix that as soon as possible to avoid forget it later... I will do better on my next PR

No worries I feel the same way.

nickofthyme

LGTM, Tested locally, with firefox and chrome. Tested fit function and banded stories.

Verified #766 and #450 are fixed

# [21.0.0](v20.0.2...v21.0.0) (2020-08-10) ### Bug Fixes * update dep vulnerabilities, minimist and kind-of ([#763](#763)) ([4455281](4455281)) * **legend:** fix color anchor, add action context, fix action padding ([#774](#774)) ([4590a22](4590a22)) * **tooltip:** placement with left/top legends and single bars ([#771](#771)) ([e576b26](e576b26)), closes [#769](#769) [#770](#770) ### Features * streamgraph and fit functions on stacked charts ([#751](#751)) ([268fcc0](268fcc0)), closes [#766](#766) [#715](#715) [#450](#450) ### BREAKING CHANGES * the first parameter of `PointStyleAccessor` and `BarStyleAccessor` callbacks is changed from `RawDataSeriesDatum` to `DataSeriesDatum`. `stackAsPercentage` prop is replaced by `stackMode` that accept one `StackMode`.

markov00 · 2020-08-10T20:52:12Z

🎉 This PR is included in version 21.0.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

# [21.0.0](elastic/elastic-charts@v20.0.2...v21.0.0) (2020-08-10) ### Bug Fixes * update dep vulnerabilities, minimist and kind-of ([opensearch-project#763](elastic/elastic-charts#763)) ([843554f](elastic/elastic-charts@843554f)) * **legend:** fix color anchor, add action context, fix action padding ([opensearch-project#774](elastic/elastic-charts#774)) ([262f8d2](elastic/elastic-charts@262f8d2)) * **tooltip:** placement with left/top legends and single bars ([opensearch-project#771](elastic/elastic-charts#771)) ([75533b1](elastic/elastic-charts@75533b1)), closes [opensearch-project#769](elastic/elastic-charts#769) [opensearch-project#770](elastic/elastic-charts#770) ### Features * streamgraph and fit functions on stacked charts ([opensearch-project#751](elastic/elastic-charts#751)) ([6f6a8cb](elastic/elastic-charts@6f6a8cb)), closes [opensearch-project#766](elastic/elastic-charts#766) [opensearch-project#715](elastic/elastic-charts#715) [opensearch-project#450](elastic/elastic-charts#450) ### BREAKING CHANGES * the first parameter of `PointStyleAccessor` and `BarStyleAccessor` callbacks is changed from `RawDataSeriesDatum` to `DataSeriesDatum`. `stackAsPercentage` prop is replaced by `stackMode` that accept one `StackMode`.

markov00 added 5 commits July 15, 2020 12:28

refactor: remove unnecessary code

1e21b8d

refactor: code cleanup on series counts

a40fcc7

Merge branch 'master' into 2020_07_15-stack_mode

d3d9b61

refactor: add d3 stack order and stack mode on stacked charts

6220e49

BREAKING CHANGE: PointStyleAccessor and BarStyleAccessor now calls the method with a DataSeriesDatum not with a RawDataSeriesDatum

markov00 force-pushed the 2020_07_15-stack_mode branch from 545cdde to 6220e49 Compare July 22, 2020 15:49

markov00 added 4 commits July 22, 2020 18:01

Merge branch 'master' into 2020_07_15-stack_mode

11b2abe

docs: update API signature

2b73892

refactor: fix typings

cbb1ccd

feat: add various stackModes to bar and area charts

bb2191f

the `stackMode` was introduced to apply percentage, wiggle and silhouette offset for stacking charts. BREAKING CHANGE: `stackAsPercentage` prop was replaced by `stackMode` that accept one `StackModes`. fix elastic#715

nickofthyme mentioned this pull request Jul 23, 2020

ci: add prettier back 🎉 #764

Merged

5 tasks

markov00 added 10 commits July 27, 2020 16:48

fix: add streamgraph example and fix few screenshots

57cca57

fix: fitting function should respect new datum format

434e225

test: update screenshot tests

5d0072f

Merge branch 'master' into 2020_07_15-stack_mode

e7ced6c

refactor: reset playground to its default

2af6f80

test: unskip test

d35f124

refactor: rename file

4028d7c

refactor: moved new data sample to datasample folder

3f3e5cd

test: cleanup tests

6c2b913

refactor: rename StackModes to StackMode

fdb00ea

markov00 added :data Data/series/scales related issue :specs Chart specifications related issue :xy Bar/Line/Area chart related enhancement New feature or request labels Jul 28, 2020

test: update screenshot based on StackMode name change

259efec

markov00 marked this pull request as ready for review July 28, 2020 14:24

markov00 requested a review from nickofthyme July 28, 2020 14:24

test: update last two screenshots

aff9ecd

nickofthyme reviewed Jul 28, 2020

View reviewed changes

src/chart_types/xy_chart/utils/specs.ts Show resolved Hide resolved

src/chart_types/xy_chart/rendering/rendering.test.ts Show resolved Hide resolved

nickofthyme reviewed Jul 28, 2020

View reviewed changes

markov00 added 3 commits July 29, 2020 10:38

refactor: remove comments and cleanup code

6919182

refactor: add comments on getDatumYValue

3d99441

refactor: hasOffsetDifferentThanZero fn rename

5a8b6a0

markov00 added 4 commits July 29, 2020 14:45

test: add storybook test for fitted stacked charts

63b29a6

test: remove unused knob from story

7e50ea1

refactor: rollback changes to playground

0517701

refactor: align clipping ranges on line and areas

df312fd

refactor: dry up some code

af500d8

nickofthyme approved these changes Jul 29, 2020

View reviewed changes

test: remove unnecessary test

0a39b18

markov00 merged commit 268fcc0 into elastic:master Jul 29, 2020

markov00 added the released Issue released publicly label Aug 10, 2020

nickofthyme mentioned this pull request Sep 16, 2020

Line series with different x values are hidden #825

Closed

markov00 deleted the 2020_07_15-stack_mode branch November 25, 2020 11:48

This was referenced Nov 1, 2023

Do not force stacking on single histogram series #2223

Closed

fix(BarSeries): ignore histogram mode in determining stacked series #2225

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: wiggle and silhouette stacks #751

feat: wiggle and silhouette stacks #751

markov00 commented Jul 15, 2020 •

edited

Loading

codecov-commenter commented Jul 22, 2020 •

edited

Loading

nickofthyme left a comment

nickofthyme left a comment

markov00 commented Jul 29, 2020

nickofthyme commented Jul 29, 2020

nickofthyme left a comment

markov00 commented Aug 10, 2020

feat: wiggle and silhouette stacks #751

feat: wiggle and silhouette stacks #751

Conversation

markov00 commented Jul 15, 2020 • edited Loading

Summary

Additional changes

BREAKING CHANGE:

Checklist

codecov-commenter commented Jul 22, 2020 • edited Loading

Codecov Report

nickofthyme left a comment

Choose a reason for hiding this comment

nickofthyme left a comment

Choose a reason for hiding this comment

markov00 commented Jul 29, 2020

nickofthyme commented Jul 29, 2020

nickofthyme left a comment

Choose a reason for hiding this comment

markov00 commented Aug 10, 2020

markov00 commented Jul 15, 2020 •

edited

Loading

codecov-commenter commented Jul 22, 2020 •

edited

Loading